Skip to content

test: enable 8 already-passing Node compat tests#33588

Open
nathanwhitbot wants to merge 10 commits intodenoland:mainfrom
nathanwhitbot:fix/node-compat-iter48
Open

test: enable 8 already-passing Node compat tests#33588
nathanwhitbot wants to merge 10 commits intodenoland:mainfrom
nathanwhitbot:fix/node-compat-iter48

Conversation

@nathanwhitbot
Copy link
Copy Markdown
Contributor

Summary

Enables 8 Node.js compat tests that already pass on main without any code changes. No production code touched — just tests/node_compat/config.jsonc entries.

  • parallel/test-cluster-send-deadlock.js
  • parallel/test-cluster-worker-isdead.js
  • parallel/test-http2-large-writes-session-memory-leak.js
  • parallel/test-http2-serve-file.js
  • parallel/test-http2-server-push-stream.js
  • parallel/test-http2-server-unknown-protocol.js
  • parallel/test-https-server-async-dispose.js
  • parallel/test-watch-mode-kill-signal-default.mjs

Test plan

  • Each test verified via cargo test --test node_compat -- <name> (8/8 pass)
  • Re-ran each test 3× to confirm stability — all stable

All 8 tests pass against current main with no code changes; this just
adds them to config.jsonc so they're tracked. Each was verified via
`cargo test --test node_compat -- <name>` and re-run three times to
rule out flakiness.

  parallel/test-cluster-send-deadlock.js
  parallel/test-cluster-worker-isdead.js
  parallel/test-http2-large-writes-session-memory-leak.js
  parallel/test-http2-serve-file.js
  parallel/test-http2-server-push-stream.js
  parallel/test-http2-server-unknown-protocol.js
  parallel/test-https-server-async-dispose.js
  parallel/test-watch-mode-kill-signal-default.mjs
Copy link
Copy Markdown
Contributor Author

@nathanwhitbot nathanwhitbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Config-only — 8 additive {} entries, all alphabetically placed correctly.

The failing test node_compat debug linux-x86_64 is an unrelated flake on
test-child-process-stdio-reuse-readable-stdio.js (an existing enabled
test, not one of the 8 added here). Failure mode: /usr/bin/head: error reading 'standard input': Resource temporarily unavailable followed by an
AssertionError — classic head -c N pipe-read race. cc @nathanwhit for a
rerun.

Otherwise LGTM.

@nathanwhitbot nathanwhitbot force-pushed the fix/node-compat-iter48 branch from 96b6c29 to a8c5359 Compare April 28, 2026 00:26
Copy link
Copy Markdown
Contributor

@fibibot fibibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 8 net config-only additions, body and diff match. Composition makes sense: 2 cluster tests presumably unblocked by #33493 (cluster.fork) landing on main, 4 http2 tests likely unblocked by the recent ext/node http2 churn (#33518/#33520/#33561/etc.), one https/server-async-dispose, and one watch-mode test.

Spot-checked alphabetical positions:

  • test-cluster-send-deadlock.js, test-cluster-worker-isdead.js — both fit cluster's alpha range
  • test-http2-large-writes-session-memory-leak.jslarge-writes-session-memory-leak < multiheaders (the next existing entry)
  • test-http2-serve-file.js, test-http2-server-push-stream.js, test-http2-server-unknown-protocol.js — three sequential http2-server entries
  • test-https-server-async-dispose.js — fits https-server-* range
  • test-watch-mode-kill-signal-default.mjs — late in alpha order

CI fully green: 134/134 checks pass, zero fail, zero pending. All test node_compat (3 shards × 6 platforms × debug+release = 36 jobs) clean, lint debug clean on all 3 platforms, all unit / unit_node / specs / integration green.

Author's "verified 3× for stability" claim is consistent with the CI pattern — none of the 8 enrollments tripped the flake retry loop.

@nathanwhitbot
Copy link
Copy Markdown
Contributor Author

test integration (2/2) debug windows-aarch64 fail on run 25032882870 is integration::lsp::lsp_node_modules_dir. Log shows repeated npm registry server error: hyper::Error(User(Service), expected value at line 1 column 1) during the run — npm test-registry mock flake, unrelated to this PR's compat test enables. cc @nathanwhit for a rerun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants